[#639] Harden resolveAgentURI with timeout and validation#649
[#639] Harden resolveAgentURI with timeout and validation#649realproject7 merged 3 commits intomainfrom
Conversation
- Add 5s AbortController timeout for remote fetches (https/ipfs) - Check res.ok before parsing response body - Enforce 50KB size limit on all payloads (remote, data: URI, raw JSON) - Wrap entire function in try/catch for graceful JSON parse error handling - Return empty object instead of throwing on any failure Fixes #639 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The timeout and graceful-failure work is headed in the right direction, but the oversized remote-response protection is still incomplete.
Findings
- [high] The remote size check happens only after
res.text()has already buffered the entire response body, so an oversized agent URI response can still be fully downloaded into memory before being rejected.- File:
lib/contracts/erc8004.ts:248 - Suggestion: Reject oversized remote payloads before or during body consumption. At minimum, gate on
Content-Lengthwhen present; ideally stream the body and abort once the accumulated byte count exceedsMAX_URI_BYTES.
- File:
Decision
Requesting changes because the current implementation does not yet safely handle oversized remote responses, which is part of the issue's acceptance criteria.
Check Content-Length header for early rejection, then stream body via ReadableStream reader and abort once accumulated bytes exceed 50KB — avoids buffering oversized responses into memory. Addresses T2a review feedback on PR #649. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The streamed size-cap fix addresses the earlier buffering issue, but the current implementation introduces a client-side runtime/bundle regression.
Findings
- [high]
resolveAgentURI()now usesBuffer.concat(...), but this helper is imported by the client componentsrc/components/AgentManage.tsx, so the change adds a Node-only dependency into client code.- File:
lib/contracts/erc8004.ts:267 - Suggestion: Keep the streamed decoding browser-safe. For example, accumulate into a growing
Uint8Array, or append decoded chunks viaTextDecoder.decode(value, { stream: true })and finalize once the reader completes.
- File:
Decision
Requesting changes because resolveAgentURI() is used from client-side code and the new Buffer dependency can break the runtime or bundle despite the size-limit fix itself being directionally correct.
Buffer.concat is Node-only and this module is imported by client components. Use plain Uint8Array concatenation instead. Addresses T2a review feedback on PR #649. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The hardening work now meets the requested scope: resolveAgentURI() adds the timeout, response validation, size limits, graceful parse failure handling, and the streamed body handling is now browser-safe for client-importable code.
Findings
- None.
Decision
Approving on code review. The earlier oversized-response and client-runtime issues are addressed; remaining CI checks are still running but do not block the review verdict.
Summary
AbortControlleron remote fetches (https/ipfs) prevents hangs from slow serversres.okbefore parsing bodydata:URI, raw JSON){}on any failure instead of throwingFixes #639
Tracks realproject7/agent-os#312
Test plan
{}){}(not crash){}gracefullydata:URI payloads return{}🤖 Generated with Claude Code